report a cancelled ctx read as the deadline, not end-of-stream - #595
Merged
Conversation
a deadline-limited read on a subprocess pipe spawns a worker to do the read and polls the context's cancel flag while waiting. under a single green worker the worker can finish the same instant the deadline fires: `task.is_done()` then wins the poll before the cancel is seen, so a read that came back empty only because it would have blocked was returned as a clean end-of-stream. a `read_exact` on top of it then failed with "unexpected eof" instead of deadline-exceeded, and `read_all` would stop short a chunk early. after awaiting the worker, the four process read primitives now re-check the context: if it is cancelled and nothing was read, that empty is the deadline, not eof. a read that returned real data before the deadline keeps it — only an empty-or-errored result is reclassified, so no data is dropped. the tcp read path already checked the cancel before reading and needed no change. found by verify-green-corpus: test_process_ctx diverged under one worker only (correct at the default worker count and os-thread). it now matches on all three, so its exclusion is removed and the whole corpus — 260 cases, both green worker modes — is byte-identical to os-thread again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary
a deadline-limited read on a subprocess pipe spawns a worker to do the read and
polls the context's cancel flag while waiting. under a single green worker the
worker can finish the same instant the deadline fires —
task.is_done()winsthe poll before the cancel is seen — so a read that came back empty only because
it would have blocked was returned as a clean end-of-stream.
read_exactthenfailed with "unexpected eof" instead of deadline-exceeded, and
read_allwouldstop a chunk short.
after awaiting the worker, the four process read primitives re-check the
context: if it is cancelled and nothing was read, that empty is the deadline,
not eof. real data read before the deadline is kept — only an empty-or-errored
result is reclassified, so no data is dropped. the tcp read path already checked
the cancel before reading and is unchanged.
how it was found
verify-green-corpus(the corpus-under-green harness added in #594) caught it:test_process_ctxdiverged under one worker only, correct at the default workercount and os threads. with this fix it matches on all three, so its entry in
GREEN_CORPUS_EXCLUDEis removed and the whole corpus — 260 cases, both greenworker modes — is byte-identical to os-thread again.
what was tested
make verify-green-corpusat 260/260 (both worker modes, exclude removed),make bootstrap-verifygreen,make green-tests,make memcheckclean undervalgrind.
test_process_ctxverified matching its expected output under osthreads, green default workers, and green single worker.
notes
std-only change; the process io/tcp/command regression cases are unaffected
(verified). this is the one green correctness gap the corpus harness surfaced —
part of the green-default-flip prerequisites.